Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the GitHub Actions CD workflows to refresh the pygeoapi-related materialized views as part of the staging and production deployment process (immediately after running Alembic migrations).
Changes:
- Add a staging workflow step to run
cli.cli refresh-pygeoapi-materialized-viewsafter DB migrations. - Add a production workflow step to run
cli.cli refresh-pygeoapi-materialized-viewsafter DB migrations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| .github/workflows/CD_staging.yml | Adds a post-migration step to refresh materialized views against the staging Cloud SQL database. |
| .github/workflows/CD_production.yml | Adds a post-migration step to refresh materialized views against the production Cloud SQL database. |
You can also share your feedback on Copilot code review. Take the survey.
| env: | ||
| DB_DRIVER: "cloudsql" | ||
| CLOUD_SQL_INSTANCE_NAME: "${{ secrets.CLOUD_SQL_INSTANCE_NAME }}" | ||
| CLOUD_SQL_DATABASE: "${{ vars.CLOUD_SQL_DATABASE }}" | ||
| CLOUD_SQL_USER: "${{ secrets.CLOUD_SQL_USER }}" | ||
| CLOUD_SQL_IAM_AUTH: true |
There was a problem hiding this comment.
The env block for the DB connection is now duplicated across multiple steps (migrations + refresh). This increases drift risk if one step’s DB settings change later. Consider moving these shared DB env vars to a job-level env: (or using a YAML anchor) and overriding only what differs per-step.
| - name: Refresh materialized views on production database | ||
| env: | ||
| DB_DRIVER: "cloudsql" | ||
| CLOUD_SQL_INSTANCE_NAME: "${{ secrets.CLOUD_SQL_INSTANCE_NAME }}" | ||
| CLOUD_SQL_DATABASE: "${{ vars.CLOUD_SQL_DATABASE }}" | ||
| CLOUD_SQL_USER: "${{ secrets.CLOUD_SQL_USER }}" | ||
| CLOUD_SQL_IAM_AUTH: true | ||
| run: | | ||
| uv run python -m cli.cli refresh-pygeoapi-materialized-views |
There was a problem hiding this comment.
The PR description is still the unfilled template ("Use bullet points here" under Why/How/Notes). Please update the PR description to describe the problem being solved and what changed so reviewers/operators understand the deployment impact (e.g., when/why the materialized view refresh runs).
| env: | ||
| DB_DRIVER: "cloudsql" | ||
| CLOUD_SQL_INSTANCE_NAME: "${{ secrets.CLOUD_SQL_INSTANCE_NAME }}" | ||
| CLOUD_SQL_DATABASE: "${{ vars.CLOUD_SQL_DATABASE }}" | ||
| CLOUD_SQL_USER: "${{ secrets.CLOUD_SQL_USER }}" | ||
| CLOUD_SQL_IAM_AUTH: true |
There was a problem hiding this comment.
The env block for the DB connection is now duplicated across multiple steps (migrations + refresh). This increases drift risk if one step’s DB settings change later. Consider moving these shared DB env vars to a job-level env: (or using a YAML anchor) and overriding only what differs per-step.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e64d6c8d37
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| CLOUD_SQL_USER: "${{ secrets.CLOUD_SQL_USER }}" | ||
| CLOUD_SQL_IAM_AUTH: true | ||
| run: | | ||
| uv run python -m cli.cli refresh-pygeoapi-materialized-views |
There was a problem hiding this comment.
In the production deploy workflow, this command invokes refresh-pygeoapi-materialized-views with its default non-concurrent mode (concurrently=False in cli/cli.py), which runs REFRESH MATERIALIZED VIEW and can hold exclusive locks on each materialized view for the duration of the refresh transaction. Because these exact views are the backing tables for live pygeoapi collections (core/pygeoapi-config.yml), requests hitting those endpoints during deployment can block until refresh completes, creating avoidable user-facing downtime; pass --concurrently (supported by the CLI) or otherwise avoid lock-heavy refreshes in the deploy path.
Useful? React with 👍 / 👎.
Why
This PR addresses the following problem / context:
How
Implementation summary - the following was changed / added / removed:
Notes
Any special considerations, workarounds, or follow-up work to note?